Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Explicit mapping in labels + frontend improvements #137

Merged
merged 6 commits into from
Nov 2, 2024
Merged

Conversation

dinmukhamedm
Copy link
Member

@dinmukhamedm dinmukhamedm commented Nov 1, 2024

  • Explicitly map {"label": numeric_val} instead of an array and use of indexes.
  • Responsive project cards on smaller screens
  • Copy button in formatter (spans view)
  • Update loader animation (icon)

Important

This pull request updates label mapping to use explicit key-value pairs, enhances UI with a copy button and responsive design, updates loader icons, and adjusts CSS variables for consistency.

  • Label Mapping:
    • Change from array-based to explicit key-value mapping for labels in evaluator-editor.tsx, add-label-popover.tsx, and add-label.tsx.
  • UI Enhancements:
    • Add copy button to formatter.tsx for copying formatted text.
    • Make project cards responsive on smaller screens in projects.tsx.
    • Update loader icon from Loader to Loader2 in multiple components including dataset-panel.tsx, dataset-upload.tsx, and delete-datapoints-dialog.tsx.
  • CSS Adjustments:
    • Modify CSS variables in globals.css for better visual consistency.
  • Dependency Update:
    • Update next-auth to version 4.24.10 in package.json.

This description was created by Ellipsis for b45f8c4. It will automatically update as commits are pushed.

dinmukhamedm and others added 5 commits November 1, 2024 12:25
* explicit mapping in labels

* Update frontend/components/traces/add-label.tsx

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>

---------

Co-authored-by: ellipsis-dev[bot] <65095814+ellipsis-dev[bot]@users.noreply.github.com>
Copy link
Contributor

@ellipsis-dev ellipsis-dev bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❌ Changes requested. Reviewed everything up to b45f8c4 in 1 minute and 7 seconds

More details
  • Looked at 1242 lines of code in 38 files
  • Skipped 1 files when reviewing.
  • Skipped posting 5 drafted comments based on config settings.
1. frontend/components/dataset/dataset-panel.tsx:2
  • Draft comment:
    The import of Loader has been replaced with Loader2 from lucide-react. Ensure that Loader2 is the intended icon and is available in the lucide-react package.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The import of Loader from lucide-react has been replaced with Loader2 across multiple files. This change is consistent and should be noted.
2. frontend/components/evaluator/evaluator-editor.tsx:174
  • Draft comment:
    The valueMap is now an object instead of an array. Ensure that this change is consistent across the codebase and that all usages of valueMap are updated accordingly.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change from using an array for valueMap to using an object is significant and affects multiple files. This change should be consistent across the codebase.
3. frontend/components/traces/add-label-popover.tsx:388
  • Draft comment:
    The valueMap is now an object instead of an array. Ensure that this change is consistent across the codebase and that all usages of valueMap are updated accordingly.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change from using an array for valueMap to using an object is significant and affects multiple files. This change should be consistent across the codebase.
4. frontend/components/traces/add-label.tsx:132
  • Draft comment:
    The valueMap is now an object instead of an array. Ensure that this change is consistent across the codebase and that all usages of valueMap are updated accordingly.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change from using an array for valueMap to using an object is significant and affects multiple files. This change should be consistent across the codebase.
5. frontend/components/traces/span-labels.tsx:109
  • Draft comment:
    The valueMap is now an object instead of an array. Ensure that this change is consistent across the codebase and that all usages of valueMap are updated accordingly.
  • Reason this comment was not posted:
    Confidence changes required: 50%
    The change from using an array for valueMap to using an object is significant and affects multiple files. This change should be consistent across the codebase.

Workflow ID: wflow_qhYJcYl4bXW3pdN1


Want Ellipsis to fix these issues? Tag @ellipsis-dev in a comment. You can customize Ellipsis with 👍 / 👎 feedback, review rules, user-specific overrides, quiet mode, and more.

@@ -61,6 +63,14 @@ export default function Formatter({
return value;
};

const copyToClipboard = () => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This functionality duplicates existing clipboard and toast operations found in project-api-keys.tsx. Consider refactoring to use a common utility function.

@dinmukhamedm dinmukhamedm merged commit 67a36ba into main Nov 2, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants